Fix Supabase key validation and consolidate frontend state management by Wirasm · Pull Request #232 · coleam00/Archon

Wirasm · 2025-08-15T21:30:36Z

Pull Request

Summary

This PR addresses a critical configuration issue where users accidentally use Supabase ANON (public) keys instead of SERVICE keys, causing database write failures. It adds backend validation to detect and prevent this misconfiguration, while also simplifying frontend state management following KISS principles.

Changes Made

Add JWT-based validation to detect anon vs service keys at backend startup
Fail fast with clear error messages when incorrect key type is detected
Consolidate frontend state management - MainLayout is now single source of truth
Remove duplicate state tracking and sessionStorage polling
Add comprehensive error display component when backend fails to start
Improve .env.example documentation with detailed key selection guidance
Add comprehensive test coverage for validation logic
Remove unused test results checking that caused 404 errors

Type of Change

Bug fix (non-breaking change which fixes an issue)
Code refactoring
Documentation update

Affected Services

Testing

All existing tests pass
Added new tests for new functionality
Manually tested affected user flows
Docker builds succeed for all services

Test Evidence

# Python validation tests
cd python && uv run pytest tests/test_supabase_validation.py -v
# Result: 11 passed

# Frontend tests
cd archon-ui-main && npm test -- --run
# Result: Test Files 4 passed (4), Tests 39 passed (39)

# Backend tests
cd python && uv run pytest tests/test_settings_api.py -v
# Result: All tests pass

Checklist

My code follows the service architecture patterns
If using an AI coding assistant, I used the CLAUDE.md rules
I have added tests that prove my fix/feature works
All new and existing tests pass locally
My changes generate no new warnings
I have updated relevant documentation
I have verified no regressions in existing features

Breaking Changes

None - the system remains backward compatible while adding validation warnings.

Additional Notes

Key Improvements:

User Protection: Prevents a very common Supabase configuration mistake
Clear Guidance: Detailed error messages with exact steps to fix
Simplified State: Removed ~50 lines of complex state synchronization code
Better Performance: Eliminated polling overhead

Files Added:

archon-ui-main/src/components/BackendStartupError.tsx - Error display component
python/tests/test_supabase_validation.py - Comprehensive validation tests

Critical Fixes Applied:

Used python-jose (already in dependencies) instead of requiring PyJWT
Made unknown key roles fail fast per alpha principles
Fixed JWT validation to properly skip all checks when only examining role

The implementation follows the "warn but don't break" approach - providing helpful validation while maintaining backward compatibility.

- Add backend validation to detect and warn about anon vs service keys - Prevent startup with incorrect Supabase key configuration - Consolidate frontend state management following KISS principles - Remove duplicate state tracking and sessionStorage polling - Add clear error display when backend fails to start - Improve .env.example documentation with detailed key selection guide - Add comprehensive test coverage for validation logic - Remove unused test results checking to eliminate 404 errors The implementation now warns users about key misconfiguration while maintaining backward compatibility. Frontend state is simplified with MainLayout as the single source of truth for backend status.

- Use python-jose (already in dependencies) instead of PyJWT for JWT decoding - Make unknown Supabase key roles fail fast per alpha principles - Skip all JWT validations (not just signature) when checking role - Update tests to expect failure for unknown roles Fixes: - No need to add PyJWT dependency - python-jose provides JWT functionality - Unknown key types now raise ConfigurationError instead of warning - JWT decode properly skips all validations to only check role claim

Wirasm · 2025-08-15T21:33:28Z

resolves #223

Wirasm · 2025-08-15T21:34:18Z

resolves #204

coleam00 · 2025-08-16T11:26:17Z

@Wirasm This is great, much needed so thank you! However, when I start up Archon on this PR I get this error in the Archon-UI container:

2025-08-16 06:23:51 /usr/local/bin/docker-entrypoint.sh: exec: line 11: /app/start.sh: not found

Might be best to not change the Dockerfile for the frontend.

- Rolled back to match main branch Dockerfile - Removed 3-second sleep script that was added for backend readiness - Container now runs npm directly without intermediate script - Tested and verified all services start correctly without the delay

Wirasm · 2025-08-18T09:37:48Z

@coleam00 please test it again

Wirasm · 2025-08-18T09:47:49Z

solves: #286

coleam00

Perfect now!

…ions-health Feat/ghcr integrations health

…237) * Fix: Add stale workflow cleanup and defense-in-depth error handling Problem: Workflows could get stuck in "running" state indefinitely when the async generator disconnected but the AI subprocess continued working. This blocked new workflow invocations with "Workflow already running" errors. Root cause: No cleanup mechanism existed for workflows that failed to complete due to disconnection between the executor and the Claude SDK. Solution (defense-in-depth): 1. Activity-based staleness detection: Workflows inactive for 15+ minutes are auto-failed when a new workflow is triggered on the same conversation 2. Top-level error handling: All errors in workflow execution are caught and the workflow is properly marked as failed (prevents stuck state) 3. Manual cancel command: /workflow cancel lets users force-fail stuck workflows immediately Changes: - Add last_activity_at column via migration for staleness tracking - Add updateWorkflowActivity() to track activity during execution - Add staleness check before blocking concurrent workflows - Wrap workflow execution in try-catch to ensure failure is recorded - Add /workflow cancel subcommand to command handler - Update test to match new error handling behavior Fixes #232 * docs: Add /workflow cancel command to documentation * Improve error handling and add comprehensive tests for stale workflow cleanup Error handling improvements: - Add workflow ID and error context to updateWorkflowActivity logs - Add stack trace, error name, and cause to top-level catch block - Separate DB failure recording from file logging for clearer error messages - Add try-catch around staleness cleanup with user-facing error message - Check sendCriticalMessage return value and log when user not notified Test coverage additions: - Add staleness detection tests (stale vs non-stale, fallback to started_at) - Add /workflow cancel command tests - Add updateWorkflowActivity function tests (including non-throwing behavior) All 845 tests pass, type-check clean, lint clean.

* Fix: PR worktrees use actual branch for same-repo PRs (#232) When creating a worktree for a PR review, the system was always creating a synthetic branch (`pr-N-review`) instead of using the PR's actual branch. This caused confusion when pushing changes back to the PR, as commits went to an orphan branch instead of updating the PR. Changes: - Add `isForkPR` field to IsolationHints and IsolationRequest - Add fork detection in GitHub adapter (compares head.repo vs base.repo) - Update createFromPR() to use actual branch for same-repo PRs - Fork PRs still use synthetic branch (can't push to forks anyway) - Update generateBranchName() to return actual PR branch for same-repo PRs - Add tests for same-repo vs fork PR behavior Fixes #232 * docs: Update PR worktree behavior documentation for fork vs same-repo PRs * Address PR review findings: error handling and test coverage 1. **Error handling improvements**: - Add `prFetchFailed` flag to `IsolationHints` for degraded mode detection - Distinguish transient vs non-transient failures in PR fetch error handling - Make upstream tracking failure non-fatal (worktree still usable) 2. **Documentation**: - Add comment clarifying deleted fork edge case (null head.repo) 3. **Test coverage**: - Add fork detection unit tests (same-repo, fork, deleted fork, case sensitivity) - Add orchestrator integration tests for isForkPR pass-through Addresses review feedback from PR #238. * Simplify error handling in PR fetch logic - Remove code duplication in logging branches - Use dynamic function selection for log level - Remove redundant boolean fields from log output - Consolidate comments

…oleam00#237) * Fix: Add stale workflow cleanup and defense-in-depth error handling Problem: Workflows could get stuck in "running" state indefinitely when the async generator disconnected but the AI subprocess continued working. This blocked new workflow invocations with "Workflow already running" errors. Root cause: No cleanup mechanism existed for workflows that failed to complete due to disconnection between the executor and the Claude SDK. Solution (defense-in-depth): 1. Activity-based staleness detection: Workflows inactive for 15+ minutes are auto-failed when a new workflow is triggered on the same conversation 2. Top-level error handling: All errors in workflow execution are caught and the workflow is properly marked as failed (prevents stuck state) 3. Manual cancel command: /workflow cancel lets users force-fail stuck workflows immediately Changes: - Add last_activity_at column via migration for staleness tracking - Add updateWorkflowActivity() to track activity during execution - Add staleness check before blocking concurrent workflows - Wrap workflow execution in try-catch to ensure failure is recorded - Add /workflow cancel subcommand to command handler - Update test to match new error handling behavior Fixes coleam00#232 * docs: Add /workflow cancel command to documentation * Improve error handling and add comprehensive tests for stale workflow cleanup Error handling improvements: - Add workflow ID and error context to updateWorkflowActivity logs - Add stack trace, error name, and cause to top-level catch block - Separate DB failure recording from file logging for clearer error messages - Add try-catch around staleness cleanup with user-facing error message - Check sendCriticalMessage return value and log when user not notified Test coverage additions: - Add staleness detection tests (stale vs non-stale, fallback to started_at) - Add /workflow cancel command tests - Add updateWorkflowActivity function tests (including non-throwing behavior) All 845 tests pass, type-check clean, lint clean.

* Fix: PR worktrees use actual branch for same-repo PRs (coleam00#232) When creating a worktree for a PR review, the system was always creating a synthetic branch (`pr-N-review`) instead of using the PR's actual branch. This caused confusion when pushing changes back to the PR, as commits went to an orphan branch instead of updating the PR. Changes: - Add `isForkPR` field to IsolationHints and IsolationRequest - Add fork detection in GitHub adapter (compares head.repo vs base.repo) - Update createFromPR() to use actual branch for same-repo PRs - Fork PRs still use synthetic branch (can't push to forks anyway) - Update generateBranchName() to return actual PR branch for same-repo PRs - Add tests for same-repo vs fork PR behavior Fixes coleam00#232 * docs: Update PR worktree behavior documentation for fork vs same-repo PRs * Address PR review findings: error handling and test coverage 1. **Error handling improvements**: - Add `prFetchFailed` flag to `IsolationHints` for degraded mode detection - Distinguish transient vs non-transient failures in PR fetch error handling - Make upstream tracking failure non-fatal (worktree still usable) 2. **Documentation**: - Add comment clarifying deleted fork edge case (null head.repo) 3. **Test coverage**: - Add fork detection unit tests (same-repo, fork, deleted fork, case sensitivity) - Add orchestrator integration tests for isForkPR pass-through Addresses review feedback from PR coleam00#238. * Simplify error handling in PR fetch logic - Remove code duplication in logging branches - Use dynamic function selection for log level - Remove redundant boolean fields from log output - Consolidate comments

…oleam00#237) * Fix: Add stale workflow cleanup and defense-in-depth error handling Problem: Workflows could get stuck in "running" state indefinitely when the async generator disconnected but the AI subprocess continued working. This blocked new workflow invocations with "Workflow already running" errors. Root cause: No cleanup mechanism existed for workflows that failed to complete due to disconnection between the executor and the Claude SDK. Solution (defense-in-depth): 1. Activity-based staleness detection: Workflows inactive for 15+ minutes are auto-failed when a new workflow is triggered on the same conversation 2. Top-level error handling: All errors in workflow execution are caught and the workflow is properly marked as failed (prevents stuck state) 3. Manual cancel command: /workflow cancel lets users force-fail stuck workflows immediately Changes: - Add last_activity_at column via migration for staleness tracking - Add updateWorkflowActivity() to track activity during execution - Add staleness check before blocking concurrent workflows - Wrap workflow execution in try-catch to ensure failure is recorded - Add /workflow cancel subcommand to command handler - Update test to match new error handling behavior Fixes coleam00#232 * docs: Add /workflow cancel command to documentation * Improve error handling and add comprehensive tests for stale workflow cleanup Error handling improvements: - Add workflow ID and error context to updateWorkflowActivity logs - Add stack trace, error name, and cause to top-level catch block - Separate DB failure recording from file logging for clearer error messages - Add try-catch around staleness cleanup with user-facing error message - Check sendCriticalMessage return value and log when user not notified Test coverage additions: - Add staleness detection tests (stale vs non-stale, fallback to started_at) - Add /workflow cancel command tests - Add updateWorkflowActivity function tests (including non-throwing behavior) All 845 tests pass, type-check clean, lint clean.

* Fix: PR worktrees use actual branch for same-repo PRs (coleam00#232) When creating a worktree for a PR review, the system was always creating a synthetic branch (`pr-N-review`) instead of using the PR's actual branch. This caused confusion when pushing changes back to the PR, as commits went to an orphan branch instead of updating the PR. Changes: - Add `isForkPR` field to IsolationHints and IsolationRequest - Add fork detection in GitHub adapter (compares head.repo vs base.repo) - Update createFromPR() to use actual branch for same-repo PRs - Fork PRs still use synthetic branch (can't push to forks anyway) - Update generateBranchName() to return actual PR branch for same-repo PRs - Add tests for same-repo vs fork PR behavior Fixes coleam00#232 * docs: Update PR worktree behavior documentation for fork vs same-repo PRs * Address PR review findings: error handling and test coverage 1. **Error handling improvements**: - Add `prFetchFailed` flag to `IsolationHints` for degraded mode detection - Distinguish transient vs non-transient failures in PR fetch error handling - Make upstream tracking failure non-fatal (worktree still usable) 2. **Documentation**: - Add comment clarifying deleted fork edge case (null head.repo) 3. **Test coverage**: - Add fork detection unit tests (same-repo, fork, deleted fork, case sensitivity) - Add orchestrator integration tests for isForkPR pass-through Addresses review feedback from PR coleam00#238. * Simplify error handling in PR fetch logic - Remove code duplication in logging branches - Use dynamic function selection for log level - Remove redundant boolean fields from log output - Consolidate comments

Wirasm added 2 commits August 16, 2025 00:10

Wirasm requested a review from coleam00 August 15, 2025 21:34

coleam00 approved these changes Aug 18, 2025

View reviewed changes

Wirasm merged commit 667cae2 into main Aug 18, 2025
7 checks passed

Wirasm deleted the fix/supabase-key-validation-and-state-consolidation branch August 18, 2025 18:19

POWERFULMOVES added a commit to POWERFULMOVES/PMOVES-Archon that referenced this pull request Feb 12, 2026

Merge pull request coleam00#232 from POWERFULMOVES/feat/ghcr-integrat…

ccb53ff

…ions-health Feat/ghcr integrations health

TimothiousAI mentioned this pull request Apr 26, 2026

fix(phase-executor): build-all node 120s timeout kills 5 of 6 harness dispatches #1415

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Supabase key validation and consolidate frontend state management#232

Fix Supabase key validation and consolidate frontend state management#232
Wirasm merged 3 commits intomainfrom
fix/supabase-key-validation-and-state-consolidation

Wirasm commented Aug 15, 2025

Uh oh!

Wirasm commented Aug 15, 2025

Uh oh!

Wirasm commented Aug 15, 2025

Uh oh!

coleam00 commented Aug 16, 2025

Uh oh!

Wirasm commented Aug 18, 2025 •

edited

Loading

Uh oh!

Wirasm commented Aug 18, 2025

Uh oh!

coleam00 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Wirasm commented Aug 15, 2025

Pull Request

Summary

Changes Made

Type of Change

Affected Services

Testing

Test Evidence

Checklist

Breaking Changes

Additional Notes

Key Improvements:

Files Added:

Critical Fixes Applied:

Uh oh!

Wirasm commented Aug 15, 2025

Uh oh!

Wirasm commented Aug 15, 2025

Uh oh!

coleam00 commented Aug 16, 2025

Uh oh!

Wirasm commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Wirasm commented Aug 18, 2025

Uh oh!

coleam00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Wirasm commented Aug 18, 2025 •

edited

Loading